refactor(e2e): use error-returning assertions - #9238

e2e/vmss.go:335

• When addPodIPConfigsForAzureCNI fails, the error is returned without context. Wrapping it will make it clearer which step failed when VMSS creation errors bubble up.

		if err != nil {
			return armcompute.VirtualMachineScaleSet{}, err
		}

e2e/vmss.go:223

• The nil-guard in createVMSSModel returns a generic error message, which makes it hard to debug which required Scenario field is missing. Including the key required fields in the message would make failures actionable without needing extra logs.

This issue also appears on line 333 of the same file.

		return armcompute.VirtualMachineScaleSet{}, fmt.Errorf("scenario runtime is incomplete for VMSS model creation")

e2e/test_helpers.go:94

• RunScenario calls s.T.Error(err) when runScenario returns an error, but s.T is assigned inside runScenario; if runScenario fails before setting it, this will panic. Report the error on the current test handle (t) instead.

	if config.Config.DisableScriptless || scriptlessUnsupported(s) {
		if err := runScenario(t, s); err != nil {
			s.T.Error(err)
		}
		return

e2e/test_helpers.go:103

• Same issue as above: s.T may be nil when handling errors from runScenario. Using t.Error(err) avoids a nil dereference and ensures the failure is attributed to the correct test/subtest.

	if s.Runtime == nil {
		s.Runtime = &ScenarioRuntime{}
	}
	s.Runtime.EnableScriptlessNBCCSECmd = true
	if err := runScenario(t, s); err != nil {
		s.T.Error(err)
	}

e2e/test_helpers.go:86

• RunScenario reports errors via s.T.Error(err), but s.T is not initialized until inside runScenario (and in the VHDCreation subtest it may remain nil on early failure), which can panic and hide the real error. Use the local *testing.T (the subtest's t) to report the failure instead.

This issue also appears in the following locations of the same file:

• line 90
• line 97

		t.Run("VHDCreation", func(t *testing.T) {
			t.Parallel()
			if err := runScenarioWithPreProvision(t, s); err != nil {
				s.T.Error(err)
			}

e2e/test_helpers.go:92

• RunScenario logs errors via s.T.Error(err), but s.T is only assigned inside runScenario. In this branch, an early failure from runScenario can occur before s.T is set, leading to a nil-pointer panic. Use the t argument for reporting.

			s.T.Error(err)

e2e/test_helpers.go:102

• RunScenario uses s.T.Error(err) here, but s.T may still be nil if runScenario returns an error before it assigns s.T = t. This can panic and hide the original error. Prefer t.Error(err) for reporting from the test boundary.

		s.T.Error(err)

e2e/exec.go:180

• execScriptOnVMForScenario dereferences s.Runtime.VM without validating that s/runtime/VM are non-nil. With the new error-returning API, this should return a descriptive error rather than panic if called before ScenarioRuntime is fully initialized (or if VM creation failed).

func execScriptOnVMForScenario(ctx context.Context, s *Scenario, cmd string) (*podExecResult, error) {
	s.T.Helper()
	result, err := execScriptOnVm(ctx, s, s.Runtime.VM, cmd)
	if err != nil {
		return nil, fmt.Errorf("execute command %q on VM: %w", cmd, err)
	}
	return result, nil

e2e/vmss.go:215

• CustomDataWithNBCCmdHack silently succeeds even when the decoded custom data does not contain the "#hotfix-marker" placeholder (strings.Replace would be a no-op). That would produce custom data that never downloads/executes the hotfix binary, and the failure would show up later in harder-to-debug ways. Consider explicitly validating that the marker exists and returning an error if not.

	binaryDownloadCmd := fmt.Sprintf("curl -fSL --retry 10 --retry-delay 2 --retry-connrefused \"%s\" -o /opt/azure/containers/aks-node-controller-hotfix && chmod +x /opt/azure/containers/aks-node-controller-hotfix", binaryURL)
	customData = strings.Replace(string(decoded), "#hotfix-marker", binaryDownloadCmd, -1)
	return base64.StdEncoding.EncodeToString([]byte(customData)), nil

e2e/exec.go:167

• execOnVMForScenarioOnUnprivilegedPod now returns errors, but it can still panic if s, s.T, s.Runtime, s.Runtime.Kube, or s.Runtime.VM is nil (it dereferences them before any validation). Since this refactor is explicitly trying to propagate structured errors, add a defensive nil-guard at the top and return an error instead of panicking.

This issue also appears on line 174 of the same file.

func execOnVMForScenarioOnUnprivilegedPod(ctx context.Context, s *Scenario, cmd string) (*podExecResult, error) {
	s.T.Helper()
	nonHostPod, err := s.Runtime.Kube.GetPodNetworkDebugPodForNode(ctx, s.Runtime.VM.KubeName)
	if err != nil {
		return nil, fmt.Errorf("get non-host debug pod: %w", err)
	}
	execResult, err := execOnUnprivilegedPod(ctx, s.Runtime.Kube, nonHostPod.Namespace, nonHostPod.Name, cmd)

e2e/vmss.go:95

• CreateVMSSWithRetry now returns a structured error when NodeResourceGroup is nil, but the cleanup registered in ConfigureAndCreateVMSS still unconditionally calls cleanupVMSS, which eventually dereferences *s.Runtime.Cluster.Model.Properties.NodeResourceGroup (e.g., in deleteVMSS). In the rare-but-known “incomplete cluster model” cases this will reintroduce a nil-pointer panic during cleanup, masking the intended error propagation.

Consider guarding the cleanup call (or making cleanupVMSS/deleteVMSS tolerate a missing node RG) so failures return cleanly without panicking.

	s.T.Cleanup(func() {
		if vm != nil {
			defer cleanupBastionTunnel(vm.SSHClient)
		}
		cleanupVMSS(ctx, s, vm)
	})